Migrate HTTP layer to utopia-php/client#134
Conversation
Replace the hand-rolled cURL in Adapter::request()/requestMulti() with the utopia-php/client PSR-18 client, and have every adapter consume the PSR-7 response directly (getStatusCode/getBody/getHeaderLine) instead of the old custom result array. - request() builds a PSR-7 request via the request factory (json/form/ multipart chosen from Content-Type) and returns ResponseInterface. - requestMulti() sends sequentially over one HTTP/2, connection-reused client (APNs requires HTTP/2) and returns responses in request order; callers map results by position. - Mailgun attachments now use Psr7 multipart Part::file instead of curl_file_create; fixed a latent Vonage associative-header bug. - Test stubs (SESStub/ResendStub) return Utopia\Psr7\Response. utopia-php/client requires PHP 8.4, so bump composer (require + platform), the Dockerfile to php:8.4, and phpstan to ^2 (needed to parse the client's 8.4 syntax). Also fix a PHP 8.4 implicit-nullable deprecation in JWT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryReplaces the hand-rolled cURL layer across all 22 adapters with
Confidence Score: 3/5The core HTTP migration is correct and well-structured, but the error-reporting paths in four email adapters have a systematic double-read of the response body stream that will silently drop real error text with any non-seekable stream. The double-read pattern appears in four separate adapters and within SES's errorMessage helper itself. In each case the raw-body fallback will return an empty string if the stream was consumed by the preceding json_decode call, causing callers to receive 'Unknown error' instead of the actual provider message. This is a fresh regression introduced by the move from pre-decoded response arrays to streaming bodies. SES::errorMessage / isTemplateMissing, Resend::extractErrorMessage, Mailgun error branch, and Sendgrid error branch. The FCM access-token fetch also warrants a status-code guard. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ); | ||
|
|
||
| $accessToken = $token['response']['access_token']; | ||
| $accessToken = \json_decode((string) $token->getBody(), true)['access_token'] ?? null; |
There was a problem hiding this comment.
If Google returns a 4xx/5xx to the JWT exchange (e.g. an invalid private key or expired credential), json_decode on the error body won't contain access_token, so $accessToken silently becomes null. The string interpolation "Authorization: Bearer {$accessToken}" then sends Bearer (empty token) to every FCM push, which all fail with a 401. Adding an explicit status check before consuming $accessToken would surface the credential error immediately rather than generating hundreds of silent push failures.
| $client = $this->client($timeout, $connectTimeout, multi: true); | ||
|
|
||
| \curl_setopt($ch, CURLOPT_URL, $urls[$i]); | ||
| \curl_setopt($ch, CURLOPT_POSTFIELDS, $bodies[$i]); | ||
| \curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); | ||
| \curl_setopt($ch, CURLOPT_PRIVATE, $i); | ||
| \curl_multi_add_handle($mh, \curl_copy_handle($ch)); | ||
| $responses = []; | ||
| foreach ($urls as $i => $url) { | ||
| $responses[] = $client->sendRequest($this->buildRequest($method, $url, $headers, $bodies[$i])); | ||
| } |
There was a problem hiding this comment.
requestMulti is now sequential, not concurrent
The old curl_multi_exec implementation dispatched all requests at the same time and collected results as they completed. The new loop calls $client->sendRequest() synchronously for each URL, awaiting each response before sending the next. For APNS and FCM batches that can reach 5,000 recipients, throughput under load will be noticeably lower than before, even with HTTP/2 connection reuse eliminating per-request TLS handshakes.
| --prefer-dist | ||
|
|
||
| FROM php:8.3.11-cli-alpine3.20 | ||
| FROM php:8.4-cli-alpine |
There was a problem hiding this comment.
php:8.3.11-cli-alpine3.20). The new tag php:8.4-cli-alpine is a floating tag: the next docker pull will silently pick up a new Alpine minor version, which can introduce different library versions or behaviour and break reproducible builds.
| FROM php:8.4-cli-alpine | |
| FROM php:8.4-cli-alpine3.21 |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Replaces the hand-rolled cURL in
Adapter::request()/Adapter::requestMulti()with theutopia-php/clientPSR-18 client. Every adapter now consumes the client's PSR-7 response directly (getStatusCode()/getBody()/getHeaderLine()) instead of the old custom['statusCode', 'response', 'headers', 'error']array.Changes
Base
Adapterrequest()builds a PSR-7 request viaUtopia\Psr7\Request\Factory(picksjson/form/multipartfrom theContent-Type) and returnsPsr\Http\Message\ResponseInterface.requestMulti()sends sequentially over a single HTTP/2, connection-reused client (APNs requires HTTP/2) and returnsResponseInterface[]in request order — callers map results by position instead of['index']/basename(['url']).ClientExceptionInterfacerather than anerrorfield.Adapters (all 22)
curl_file_createtoPsr7\Request\Multipart\Part::file.'Content-Type' => …header would have sent the body as multipart instead of form-urlencoded.utopia-php/clientrequires PHP 8.4, so this bumps:composer.jsonrequire.php(>=8.1→>=8.4) andconfig.platform.php(8.3→8.4)Dockerfile(php:8.3.11→php:8.4)phpstan/phpstan(^1→^2, needed to parse the client's PHP 8.4 syntax)Also fixes a PHP 8.4 implicit-nullable deprecation in
Helpers/JWT.php.Testing
composer lint✅ ·composer analyse(PHPStan v2, level 6) ✅SESRoutingTest+ResendRoutingTest— 26 network-free routing tests, 113 assertions ✅ (stubs returnUtopia\Psr7\Response)SMSTest(Mock → request-catcher, Docker on PHP 8.4) ✅ — verifies User-Agent, custom headers, POST, JSON body, response parsingrequestMultismoke test ✅ — 1 URL + 3 bodies → 3 orderedUtopia\Psr7\Response, all 200🤖 Generated with Claude Code